Skip to content

Conversation

scottmcm
Copy link
Member

We can just unsize the array to a slice and drop that instead, reusing the same slice loop for every array length.

As part of this (and how I originally noticed this), use PtrMetadata instead of Len for the slice length, for another step closer to being able to remove Rvalue::Len.

Demonstration that yes, every slice length gets its own loop today: https://rust.godbolt.org/z/5EsPjPWv4

@rustbot
Copy link
Collaborator

rustbot commented Dec 14, 2024

r? @BoxyUwU

rustbot has assigned @BoxyUwU.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 14, 2024
@scottmcm
Copy link
Member Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Dec 14, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 14, 2024
…<try>

Stop emitting drop loops for every array length

We can just unsize the array to a slice and drop that instead, reusing the same slice loop for every array length.

As part of this (and how I originally noticed this), use `PtrMetadata` instead of `Len` for the slice length, for another step closer to being able to remove `Rvalue::Len`.

Demonstration that yes, every slice length gets its own loop today: <https://rust.godbolt.org/z/5EsPjPWv4>
@bors
Copy link
Collaborator

bors commented Dec 14, 2024

⌛ Trying commit f6cd47a with merge 41701b3...

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Dec 14, 2024

💔 Test failed - checks-actions

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 14, 2024
We can just unsize the array to a slice and drop that instead, reusing the same loop.

As part of this (and how I originally noticed this), use `PtrMetadata` instead of `Len` for the slice length, for another step closer to being able to remove `Rvalue::Len`.
@scottmcm scottmcm force-pushed the drop-arrays-by-unsizing branch from f6cd47a to aac57cf Compare December 14, 2024 07:03
@rust-log-analyzer

This comment has been minimized.

@scottmcm
Copy link
Member Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@bors
Copy link
Collaborator

bors commented Dec 14, 2024

⌛ Trying commit aac57cf with merge 4ce3fa5...

bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 14, 2024
…<try>

Stop emitting drop loops for every array length

We can just unsize the array to a slice and drop that instead, reusing the same slice loop for every array length.

As part of this (and how I originally noticed this), use `PtrMetadata` instead of `Len` for the slice length, for another step closer to being able to remove `Rvalue::Len`.

Demonstration that yes, every slice length gets its own loop today: <https://rust.godbolt.org/z/5EsPjPWv4>
@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Dec 14, 2024

💔 Test failed - checks-actions

@lqd
Copy link
Member

lqd commented Dec 14, 2024

(These failures within opt-dist running rustc-perf aren't super clear... I'll try to see how to improve that.)

@rust-log-analyzer

This comment has been minimized.

@scottmcm scottmcm force-pushed the drop-arrays-by-unsizing branch from f1c87ea to a5f9505 Compare December 15, 2024 04:03
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
   Compiling mdbook-trpl v0.1.0 (/checkout/src/doc/book/packages/mdbook-trpl)
   Compiling rustbook v0.1.0 (/checkout/src/tools/rustbook)
    Finished `release` profile [optimized] target(s) in 25.92s
##[endgroup]
error: process didn't exit successfully: `/checkout/obj/build/bootstrap/debug/rustc /checkout/obj/build/bootstrap/debug/rustc -vV` (exit status: 101)
thread 'main' panicked at compiler/rustc_driver_impl/src/lib.rs:1587:6:
thread 'main' panicked at compiler/rustc_driver_impl/src/lib.rs:1587:6:
Unable to install ctrlc handler: System(Custom { kind: Other, error: EBADF })
   0:     0x7f5569025131 - <<std[dfe8e69f7cd2cb9c]::sys::backtrace::BacktraceLock>::print::DisplayBacktrace as core[390652ced2a3c436]::fmt::Display>::fmt
   1:     0x7f55690840b3 - core[390652ced2a3c436]::fmt::write
   2:     0x7f5569018009 - <std[dfe8e69f7cd2cb9c]::sys::pal::unix::stdio::Stderr as std[dfe8e69f7cd2cb9c]::io::Write>::write_fmt
   3:     0x7f5569024fd2 - <std[dfe8e69f7cd2cb9c]::sys::backtrace::BacktraceLock>::print
   3:     0x7f5569024fd2 - <std[dfe8e69f7cd2cb9c]::sys::backtrace::BacktraceLock>::print
   4:     0x7f5569027a31 - std[dfe8e69f7cd2cb9c]::panicking::default_hook::{closure#1}
   5:     0x7f55690277af - std[dfe8e69f7cd2cb9c]::panicking::default_hook
   6:     0x7f5564773518 - <alloc[c0599e64d5b5f875]::boxed::Box<rustc_driver_impl[efef9de6942c42c2]::install_ice_hook::{closure#0}> as core[390652ced2a3c436]::ops::function::Fn<(&dyn for<'a, 'b> core[390652ced2a3c436]::ops::function::Fn<(&'a std[dfe8e69f7cd2cb9c]::panic::PanicHookInfo<'b>,), Output = ()> + core[390652ced2a3c436]::marker::Sync + core[390652ced2a3c436]::marker::Send, &std[dfe8e69f7cd2cb9c]::panic::PanicHookInfo)>>::call
   7:     0x7f5569028448 - std[dfe8e69f7cd2cb9c]::panicking::rust_panic_with_hook
   8:     0x7f556902803e - std[dfe8e69f7cd2cb9c]::panicking::begin_panic_handler::{closure#0}
   9:     0x7f5569025759 - std[dfe8e69f7cd2cb9c]::sys::backtrace::__rust_end_short_backtrace::<std[dfe8e69f7cd2cb9c]::panicking::begin_panic_handler::{closure#0}, !>
  11:     0x7f556907fb00 - core[390652ced2a3c436]::panicking::panic_fmt
  12:     0x7f55690800f6 - core[390652ced2a3c436]::result::unwrap_failed
  13:     0x7f55646a1995 - rustc_driver_impl[efef9de6942c42c2]::main
  14:     0x55a786bda8a7 - rustc_main[22b90118f880c90b]::main
---
warning: the ICE couldn't be written to `/checkout/rustc-ice-2024-12-15T04_21_58-26907.txt`: Read-only file system (os error 30)

note: rustc 1.85.0-nightly (dd429d1e6 2024-12-15) running on x86_64-unknown-linux-gnu

note: compiler flags: -Z on-broken-pipe=kill -Z unstable-options
query stack during panic:
end of query stack

Build completed unsuccessfully in 0:00:30
---
##[group]Building bootstrap
    Finished `dev` profile [unoptimized] target(s) in 0.04s
##[endgroup]
WARN: currently no CI rustc builds have rustc debug assertions enabled. Please either set `rust.debug-assertions` to `false` if you want to use download CI rustc or set `rust.download-rustc` to `false`.
ERROR: Tool `book` was not recorded in tool state.
ERROR: Tool `nomicon` was not recorded in tool state.
ERROR: Tool `reference` was not recorded in tool state.
ERROR: Tool `rust-by-example` was not recorded in tool state.
ERROR: Tool `edition-guide` was not recorded in tool state.
ERROR: Tool `embedded-book` was not recorded in tool state.
  local time: Sun Dec 15 04:21:58 UTC 2024
  network time: Sun, 15 Dec 2024 04:21:59 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

@scottmcm
Copy link
Member Author

scottmcm commented Dec 15, 2024

I have no idea what's happening here :/

Replaced with #134326

@scottmcm scottmcm closed this Dec 15, 2024
@scottmcm scottmcm deleted the drop-arrays-by-unsizing branch December 15, 2024 05:43
@lqd
Copy link
Member

lqd commented Dec 15, 2024

@scottmcm The collector failed with collector error: Cannot read lib dir to find components (the error handling is buggy there, I'll fix it in rust-lang/rustc-perf#2021). It failed because it was trying to read the "lib" directory in the sysroot but couldn't: stage2 rustc panicked when asked to print the sysroot:

collector error: Cannot find libdir for rustc

Caused by:
    rustc failed to provide sysroot, error: exit status: 101, stderr:
    thread 'main' panicked at compiler/rustc_driver_impl/src/lib.rs:1587:6:
    Unable to install ctrlc handler: System(Custom { kind: Other, error: EBADF })
    stack backtrace:
       0: rust_begin_unwind
       1: core::panicking::panic_fmt
       2: core::result::unwrap_failed
       3: rustc_driver_impl::install_ctrlc_handler
       4: rustc_driver_impl::main
       5: rustc_main::main
    note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

    error: the compiler unexpectedly panicked. this is a bug.

    note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

    note: please make sure that you have updated to the latest nightly

    note: please attach the file at `/tmp/tmp-multistage/opt-artifacts/rustc-perf/rustc-ice-2024-12-15T17_44_07-54539.txt` to your bug report

    query stack during panic:
    end of query stack

(I've launched a try build with the other PR, to see if these errors reproduce)

bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 15, 2024
…, r=<try>

Use `PtrMetadata` instead of `Len` in slice drop shims

I tried to do a bigger change in rust-lang#134297 which didn't work, so here's the part I really wanted: Removing another use of `Len`, in favour of `PtrMetadata`.

Reusing the same reviewer from the last one:
r? BoxyUwU
@scottmcm
Copy link
Member Author

Thanks for digging in, @lqd ! I think I'm not monoing enough here, or something :/

@lqd
Copy link
Member

lqd commented Dec 16, 2024

The good news is this behavior doesn’t seem to reproduce in the #134326 subset 🎉

bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 22, 2024
…, r=saethlin

Use `PtrMetadata` instead of `Len` in slice drop shims

I tried to do a bigger change in rust-lang#134297 which didn't work, so here's the part I really wanted: Removing another use of `Len`, in favour of `PtrMetadata`.

Split into two commits where the first just adds a test, so you can look at the second commit to see how the drop shim for an array changes with this PR.

Reusing the same reviewer from the last one:
r? BoxyUwU
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 22, 2024
…, r=saethlin

Use `PtrMetadata` instead of `Len` in slice drop shims

I tried to do a bigger change in rust-lang#134297 which didn't work, so here's the part I really wanted: Removing another use of `Len`, in favour of `PtrMetadata`.

Split into two commits where the first just adds a test, so you can look at the second commit to see how the drop shim for an array changes with this PR.

Reusing the same reviewer from the last one:
r? BoxyUwU
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-perf Status: Waiting on a perf run to be completed. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants